Skip to content

Fix incorrect results from cumulative_sum and cumulative_prod with dtype=bool#2018

Merged
ndgrigorian merged 6 commits intomasterfrom
resolve-gh-2017
Feb 28, 2025
Merged

Fix incorrect results from cumulative_sum and cumulative_prod with dtype=bool#2018
ndgrigorian merged 6 commits intomasterfrom
resolve-gh-2017

Conversation

@ndgrigorian
Copy link
Collaborator

@ndgrigorian ndgrigorian commented Feb 25, 2025

This PR resolves gh-2017 by introducing missing dedicated kernels for bool->bool in the type matrix of cumulative_sum and cumulative_prod.

Also slips in the addition of int8->int8

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?
  • If this PR is a work in progress, are you opening the PR as a draft?

@ndgrigorian
Add missing kernels to arithmetic accumulators
e2d2b3f 
bool->bool and int8->int8 overloads were missing in cumulative_sum and cumulative_prod
@ndgrigorian
Add tests based on gh-2017
47239cd
@github-actions
Copy link

github-actions bot commented Feb 25, 2025 •
edited
Loading

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

@github-actions
Copy link

github-actions bot commented Feb 25, 2025

Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_550 ran successfully.
Passed: 895
Failed: 1
Skipped: 118

@coveralls
Copy link
Collaborator

coveralls commented Feb 25, 2025 •
edited
Loading

Coverage Status

coverage: 88.138% (-0.1%) from 88.233%
when pulling fa525a7 on resolve-gh-2017
into 5e1c87d on master.

@ndgrigorian ndgrigorian marked this pull request as ready for review February 25, 2025 22:23
@github-actions
Copy link

github-actions bot commented Feb 26, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_4 ran successfully.
Passed: 892
Failed: 4
Skipped: 118

antonwolfy
antonwolfy reviewed Feb 26, 2025
View reviewed changes
@github-actions
Copy link

github-actions bot commented Feb 26, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_9 ran successfully.
Passed: 894
Failed: 2
Skipped: 118

antonwolfy
antonwolfy previously approved these changes Feb 27, 2025
View reviewed changes
Copy link
Collaborator

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me, thank you @ndgrigorian

@ndgrigorian ndgrigorian disabled auto-merge February 27, 2025 23:12
@ndgrigorian
Add comment to needs_workaround in accumulators.hpp
fa525a7 
Documents the ticket reported for the JIT failure on CPU
@github-actions
Copy link

github-actions bot commented Feb 28, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_10 ran successfully.
Passed: 895
Failed: 2
Skipped: 125

antonwolfy
antonwolfy approved these changes Feb 28, 2025
View reviewed changes
Copy link
Collaborator

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you @ndgrigorian

@ndgrigorian ndgrigorian merged commit 0d0ff97 into master Feb 28, 2025
72 of 73 checks passed
@ndgrigorian ndgrigorian deleted the resolve-gh-2017 branch February 28, 2025 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antonwolfy antonwolfy antonwolfy approved these changes

@vlad-perevezentsev vlad-perevezentsev Awaiting requested review from vlad-perevezentsev vlad-perevezentsev is a code owner

@vtavana vtavana Awaiting requested review from vtavana

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dpt.cumulative_sum returns incorrect result for dtype=dpt.bool

3 participants

@ndgrigorian @coveralls @antonwolfy